Skip to content

Fix Intel crash in ThemeManager on macOS #13543

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 15 commits into from
Jul 23, 2025
Merged

Fix Intel crash in ThemeManager on macOS #13543

merged 15 commits into from
Jul 23, 2025
+9 −2

Conversation

Kaan0029
Copy link
Contributor

@Kaan0029 Kaan0029 commented Jul 14, 2025 •
edited
Loading

Closes #13536

Changes

  • Added try/catch block around themeWindowManager.install(scene) in ThemeManager
  • Prevents RuntimeException crash when native libraries are incompatible with ARM64 architecture

Testing

  • Tested on macOS ARM64 - no longer crashes on startup
  • Build passes successfully
  • Application starts and runs normally

Mandatory checks

  • I own the copyright of the code submitted and I license it under the MIT license
  • Change in CHANGELOG.md described in a way that is understandable for the average user (if change is visible to the user)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (if change is visible to the user)
  • Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
  • Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

Kaan0029 added 3 commits July 14, 2025 20:05
@Kaan0029
Fix ARM64 crash in ThemeManager on macOS
17f175d 
- Add try/catch around themeWindowManager.install() to handle RuntimeException
- Prevents crash when native libraries are incompatible with ARM64 architecture
- Fixes JabRef#13536
@Kaan0029
fix try catch error
77c3d97
@Kaan0029
Added changelog entry
be8e789
koppor
koppor requested changes Jul 15, 2025
View reviewed changes
@koppor koppor changed the title Fix ARM64 crash in ThemeManager on macOS Fix Intel crash in ThemeManager on macOS Jul 15, 2025
koppor
koppor requested changes Jul 20, 2025
View reviewed changes
Copy link
Member

@koppor koppor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please adress my comments.

@subhramit
Copy link
Member

[x] Change in CHANGELOG.md described in a way that is understandable for the average user (if change is visible to the user)

Why is this ticked if there is no changelog entry (at least at the time of writing this)?

koppor
koppor reviewed Jul 23, 2025
View reviewed changes
jabgui/src/main/java/org/jabref/gui/theme/ThemeManager.java Outdated
@@ -58,7 +58,7 @@ public class ThemeManager {
private final WorkspacePreferences workspacePreferences;
private final FileUpdateMonitor fileUpdateMonitor;
private final Consumer<Runnable> updateRunner;
private final ThemeWindowManager themeWindowManager;
private ThemeWindowManager themeWindowManager;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add @Nullable; because we don't use Optionals in class variables; do we @Siedlerchr

@koppor
Copy link
Member

koppor commented Jul 23, 2025

Follow-up issue: dukke/FXThemes#14

@koppor
Copy link
Member

koppor commented Jul 23, 2025

I think, we have a different understanding of "Manually tested" and "fixed"

If I open preferences, I get an exception:

IMG_20250723_134013_883 (Medium)

@koppor
Copy link
Member

koppor commented Jul 23, 2025

I fixed all the issues

  • unnecessary null check - library never returns null (found by ctrl+click on create and reading the 20 lines of code there)
  • preference opening and changing the mode works.

Since external contributors rely on the issue, I go ahead with merging.

@jabref-machine
Copy link
Collaborator

You ticked that you modified CHANGELOG.md, but no new entry was found there.

If you made changes that are visible to the user, please add a brief description along with the issue number to the CHANGELOG.md file. If you did not, please replace the cross ([x]) by a slash ([/]) to indicate that no CHANGELOG.md entry is necessary. More details can be found in our Developer Documentation about the changelog.

@trag-bot Trag bot
Copy link

trag-bot bot commented Jul 23, 2025

@trag-bot didn't find any issues in the code! ✅✨

@koppor koppor enabled auto-merge July 23, 2025 11:53
@koppor koppor added this pull request to the merge queue Jul 23, 2025
@koppor koppor mentioned this pull request Jul 23, 2025
Merged
4 tasks
Merged via the queue into JabRef:main with commit 4348224 Jul 23, 2025
42 of 43 checks passed
@github-actions github-actions bot mentioned this pull request Jul 23, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Siedlerchr Siedlerchr Siedlerchr left review comments

@koppor koppor koppor approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

JabRef does not start on Intel macs
5 participants
@Kaan0029 @subhramit @koppor @jabref-machine @Siedlerchr